fix: harden lifecycle cleanup and process smoke by intel352 · Pull Request #142 · GoCodeAlone/ratchet-cli

intel352 · 2026-07-16T14:01:39Z

Summary

surface provider cleanup query, row, and close failures with transition-limited diagnostics
cover service-level and startup-reconciliation shutdown behavior
remove the arbitrary ACP cancel grace while retaining bounded teardown and process reap
isolate three real-process lifecycle smokes on Linux and native Windows outside race coverage

Design

See docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-design.md.

Implementation Plan

See docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.

Scope Manifest

PR Count: 2
Tasks: 6
Status: Locked 2026-07-16T12:38:31Z
This PR: PR 1, Tasks 1-5, fix/reliability-followups
PR 2 remains the evidence-based retro and closeout after this PR is merged and released.

Changes

Cleanup dispatch joins query, scan, iteration, and Rows.Close failures; equivalent diagnostics are emitted at most once per minute and reset after recovery.
Cleanup fairness now waits for both secret deletion and durable queue-row removal.
Post-stop provider-operation reads cross the gRPC service boundary, and startup reconciliation is proven cancelable and redacted.
ACP cancel send completion proceeds directly to transport closure and child kill/reap. In-process tests retain exact-once peer-handler proof through an explicit barrier.
Three real-process tests share TestACPClientLifecycleBinarySmoke and a 30-second transition bound. Timed-out profile children are killed and reaped before failure.
Linux and existing windows-2025 CI jobs run the dedicated selector; race coverage skips only command and lifecycle binary smokes.

Security Boundary

No API, schema, dependency, runner, or secret-management surface changed.
Provider-operation cancellation errors retain stable classifications without secret names or credential values.
Cleanup logs contain structural failure context only.

Regression Proofs

Provider cleanup, fix reverted:

FAIL - row close sentinel missing after scan/iteration failure
FAIL - cleanup dispatch succeeded after database close
FAIL - equivalent cleanup failures logged repeatedly inside the suppression window

Provider cleanup, fix restored: focused tests, 30 fairness repetitions, and 10 race repetitions pass.

ACP grace restored:

--- FAIL: TestClientCancellationNoPostSendDelay
cancel watcher elapsed = 100ms, want 0s

ACP grace removed: timing test passes; cancellation count 10, lifecycle smoke count 20, and race-focused count 5 pass. The SDK scheduling assumption was backported to the design after stress exposed asynchronous peer-handler dispatch.

CI lifecycle step removed:

--- FAIL: TestCIIsolatesACPClientLifecycleBinarySmoke
workflow job missing step: Run ACP client lifecycle smoke

CI lifecycle step restored: releaseguard, dedicated smoke, and race-with-skip gates pass.

Verification

go test ./internal/daemon ./internal/acpclient ./internal/releaseguard -count=1 -timeout=15m
PASS

go test -race ./internal/daemon ./internal/acpclient ./internal/releaseguard -skip '^TestACPClientLifecycleBinarySmoke' -count=1 -timeout=20m
PASS

GOTMPDIR=$HOME/.rt GOCACHE=$HOME/.rc go test -p 1 ./... -count=1 -timeout=30m
PASS

go vet ./...
PASS

golangci-lint run --new-from-rev=origin/master
0 issues

goreleaser check
1 configuration file validated

git diff --check
PASS

Runtime Launch Transcript

Build:
$ go build -o /tmp/ratchet-lifecycle ./cmd/ratchet
exit 0

Launch, each command bounded to 10 seconds:
$ /tmp/ratchet-lifecycle --version
ratchet dev (unknown, unknown)

$ /tmp/ratchet-lifecycle provider setup list --json | jq 'length'
22

Windows cross-build:
$ GOOS=windows GOARCH=amd64 go build -o /tmp/ratchet-windows-amd64.exe ./cmd/ratchet
PE32+ executable (console) x86-64, for MS Windows

$ GOOS=windows GOARCH=arm64 go build -o /tmp/ratchet-windows-arm64.exe ./cmd/ratchet
PE32+ executable (console) Aarch64, for MS Windows

Failure-signature scrape: clean
Verdict: PASS

Native Windows lifecycle execution is required from the existing PR job before merge.

Doc-reconciliation: 1 item fixed - the design now records that ACP notification bytes reach the peer reader before asynchronous handler dispatch.

Copilot

Pull request overview

This PR strengthens lifecycle reliability across provider cleanup, provider-operation shutdown behavior, and ACP client cancellation/process smoke. It also wires CI and releaseguard checks so real-process lifecycle smokes run in dedicated Linux + native Windows steps and are excluded from race/coverage.

Changes:

Provider cleanup dispatch now returns classified errors (query/scan/iterate/close) with rate-limited logging, and tests assert durable-row completion and diagnostic behavior.
Provider-operation tests now cover service-boundary post-stop behavior and cancellation during startup reconciliation with redaction guarantees.
ACP client cancellation removes the post-send grace; real-process smokes are time-bounded, reliably kill/reap on timeout, and are isolated into dedicated CI steps guarded by releaseguard.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/releaseguard/workflow_test.go	Adds workflow contract assertions for dedicated lifecycle smoke steps and paired race-skip selector.
internal/daemon/provider_operations_test.go	Extends shutdown coverage to service boundary and startup reconciliation cancellation/redaction.
internal/daemon/provider_cleanup.go	Returns structured dispatch errors, joins row close errors, and rate-limits repeated diagnostics.
internal/daemon/provider_cleanup_test.go	Adds regression tests for joined row errors, query failure classification, log suppression, and fairness completion.
internal/acpclient/profiles_process_test.go	Updates cross-process profile lock tests to use the shared lifecycle smoke timeout and reap on failure.
internal/acpclient/fixture_test.go	Introduces shared 30s process-smoke timeout and a channel wait helper for process lifecycle tests.
internal/acpclient/client.go	Removes the fixed post-cancel grace and proceeds directly to bounded teardown/reap.
internal/acpclient/client_test.go	Adds a synctest timing regression, preserves exact-once in-process cancellation proof, and retags real-process smokes.
internal/acpclient/background_process_lock_test.go	Retags real-process profile lifecycle tests and replaces fixed waits with the shared helper/timeout.
.github/workflows/ci.yml	Adds dedicated Linux + native Windows lifecycle smoke steps and updates the race skip to exclude lifecycle smokes.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.scope-lock	Adds scope-lock hash for the implementation plan.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md	Adds the implementation plan detailing tasks, proofs, and rollback strategy.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-plan-review.md	Adds adversarial plan review report and resolutions.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-design.md	Adds approved design for cleanup diagnostics, shutdown coverage, cancellation teardown, and CI isolation.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-design-review.md	Adds adversarial design review report and resolutions.
docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-alignment.md	Adds alignment report mapping design requirements to plan tasks and scope checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

intel352 added 11 commits July 16, 2026 08:13

docs: design lifecycle reliability

dd42399

docs: resolve reliability design review

2ef7390

docs: define reliability closeout

de5a4d3

docs: plan lifecycle reliability

5c8e9c2

docs: resolve reliability plan review

1e016b0

docs: verify reliability plan alignment

c25531b

chore: lock lifecycle reliability scope

b0cfd7b

fix(daemon): surface cleanup failures

6887e11

test(daemon): cover reconcile shutdown

81840bd

fix(acp): bound process lifecycle smoke

6a9af7f

ci: isolate ACP process lifecycle smoke

1e1e8de

Copilot AI review requested due to automatic review settings July 16, 2026 14:01

Copilot started reviewing on behalf of intel352 July 16, 2026 14:02 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

Comment thread internal/acpclient/profiles_process_test.go Outdated

test(acp): bound process reap drain

eb03f01

intel352 requested a review from Copilot July 16, 2026 14:12

Copilot started reviewing on behalf of intel352 July 16, 2026 14:13 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

intel352 merged commit f1ff79a into master Jul 16, 2026
15 checks passed

intel352 deleted the fix/reliability-followups branch July 16, 2026 14:27

intel352 mentioned this pull request Jul 16, 2026

docs: close lifecycle reliability plan #143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: harden lifecycle cleanup and process smoke#142

fix: harden lifecycle cleanup and process smoke#142
intel352 merged 12 commits into
masterfrom
fix/reliability-followups

intel352 commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

intel352 commented Jul 16, 2026

Summary

Design

Implementation Plan

Scope Manifest

Changes

Security Boundary

Regression Proofs

Verification

Runtime Launch Transcript

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants